Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy function debug name onto stack for crash dumps #159

Merged
merged 4 commits into from Oct 10, 2018

Conversation

asherkin
Copy link
Member

This is a useful hack I've had sitting in my queue for a while.

Copy the to-be-invoked function's debug name into the ScriptedInvoker::Invoke stack frame so it is always available in minidumps even without heap information.

This will hopefully make it easier to track down reproduction steps for plugin-related crashes.

@asherkin asherkin requested a review from dvander October 10, 2017 10:59
Copy link
Member

@dvander dvander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a reasonable, statically-sized buffer is better. alloca() has Problems(TM).

Also avoid higher-level compiler optimisations removing the name
@asherkin
Copy link
Member Author

asherkin commented Feb 3, 2018

I've played with a few variations in https://godbolt.org/, I think alloca remains the best option all-round - the location and behaviour is a lot more predictable with compiler optimisations, and choosing a reasonable buffer size to avoid wasting stack space in all cases without truncating is a PITA.

@KyleSanderson
Copy link
Member

http://man7.org/linux/man-pages/man3/alloca.3.html

`There is no error indication if the stack frame cannot be extended.
(However, after a failed allocation, the program is likely to receive
a SIGSEGV signal if it attempts to access the unallocated space.)

   On many systems alloca() cannot be used inside the list of arguments
   of a function call, because the stack space reserved by alloca()
   would appear on the stack in the middle of the space for the function
   arguments.`

`This function is not in POSIX.1.

   There is evidence that the alloca() function appeared in 32V, PWB,
   PWB.2, 3BSD, and 4BSD.  There is a man page for it in 4.3BSD.  Linux
   uses the GNU version.`

I would avoid alloca at all costs.

@asherkin
Copy link
Member Author

asherkin commented Feb 3, 2018

As would just calling the function if it was allocating a large enough local variable... that information isn't exactly useful or relevant.

The DebugName of a function is fairly well constrained (and I believe should be well under the limit reasonable to stack alloc - which MS considers to be 1kB for an individual alloc), it just isn't ideal to allocate the full size required for the 90% of cases where it is much shorter.

@asherkin asherkin merged commit 3e44acb into master Oct 10, 2018
@Headline Headline deleted the debug-name-for-crash-dumps branch October 11, 2018 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants